Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Actions in executorselenium #10312

Closed
wants to merge 5 commits into from
Closed

Conversation

kereliuk
Copy link
Contributor

@kereliuk kereliuk commented Apr 4, 2018

This is an implementation of actions in executorselenium. Would like some feedback before I polish it up/make more changes if necessary.

Want to open a discussion with this and show one possibility for the API.


This change is Reviewable

@ghost
Copy link

ghost commented Apr 4, 2018

Build ERRORED

Started: 2018-04-04 20:57:50
Finished: 2018-04-04 21:25:10

Failing Jobs

  • lint
  • firefox:nightly
  • chrome:dev
  • tools_unittest in py27
  • wptrunner_infrastructure

View more information about this build on:

@jgraham
Copy link
Contributor

jgraham commented Apr 4, 2018

So this seems to pretty directly expose the selenium client API, which isn't defined anywhere and so is hard to accurately reproduce. This is why I want to ensure we work with non-Selenium implementations for this feature. With that in mind, I think we should make the testdriver API a builder with the same primitives as the underlying webdriver API i.e. pointer up down, move; key up and down; pause, and anything else I forgot. This is obviously more implementation work, but I'm pretty sure that putting the higher-level API in the js rather than in the backend is the way to go.

From a marionette point of view it exposes the commands needed for the W3C API, but you have to opt in to avoid the legacy implementation. This will require some changes that I'm happy to make.

@NavidZ
Copy link
Member

NavidZ commented May 14, 2018

Just wanted to get an update on this. @kereliuk are you working on this? Are you clear with what James refers to?
@jgraham regarding the shape of the API is it possible to also be able to inject multiple input at the same time (in a single frame). What we had in Chrome was something similar to this:
https://navidz.github.io/web-input-automation/#examples
that we were able to add pause and it would inject i'th entry of all those streams at the same time (i.e. in a single frame). For the starter we can definitely remove all these multiple input sources and just support one. Also since these are the raw actions do you think we need to also define some clear exceptions if say they sent a pointerup without any down before that or any other invalid input sequence?

@kereliuk
Copy link
Contributor Author

Apologies I forgot to reply to this.

Just wanted to get an update on this. @kereliuk are you working on this? Are you clear with what James refers to?

Yep, I think he has a point and we plan to move Chrome to a more generic executor to allow this.
The work for that is here, #10197 and I plan to address the comments by EOD.

Sorry for the delay on this again! Let me know if you have any other questions.

@foolip
Copy link
Member

foolip commented Sep 14, 2018

@gsnedders, will it make sense to do with now that we have ExecutorWebDriver? Does EdgeDriver or SafariDriver support the actions API?

@wpt-pr-bot wpt-pr-bot added testdriver.js wptrunner The automated test runner, commonly called through ./wpt run labels Sep 14, 2018
@foolip
Copy link
Member

foolip commented Sep 14, 2018

@NavidZ, perhaps this is overtaken by #12726? I see that Selenium is touched in that PR, at least.

@foolip foolip closed this Sep 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs infra testdriver.js wptrunner The automated test runner, commonly called through ./wpt run
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants